Sheffield | 26-ITP-Jan | Mona -Eltantawy | Sprint 1 | Data- Groups/sprint 1#1076
Sheffield | 26-ITP-Jan | Mona -Eltantawy | Sprint 1 | Data- Groups/sprint 1#1076Mona-Eltantawy wants to merge 9 commits intoCodeYourFuture:mainfrom
Conversation
…ications and to return the same values if the array has no dublications.
…, arry with nigative and positive number, arry with non numerical values and arry with decimels.
…e test and get all tests passed.
Mona-Eltantawy
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
There is another file in refractor that needs to be updated.
| // Sort without mutating original array | ||
| const sorted = [...numbers].sort((a, b) => a - b); |
There was a problem hiding this comment.
Why make a copy of numbers before sorting?
| .filter((item) => typeof item === "number") | ||
| .reduce((acc, num) => (num > acc ? num : acc), -Infinity); |
There was a problem hiding this comment.
Does your function return the value you expected from the following function calls?
findMax([NaN])
findMax([0, NaN, 1])
| test("array with non-number values, ignores non-numeric and returns max number", () => { | ||
| expect(findMax(["hey", 10, "hi", 60, 10])).toBe(60); | ||
| }); | ||
| // Given an array with only non-number values | ||
| // When passed to the max function | ||
| // Then it should return the least surprising value given how it behaves for all other inputs | ||
| test("array with only non-number values, returns -Infinity", () => { | ||
| expect(findMax(["a", "b", null, {}])).toBe(-Infinity); | ||
| }); |
There was a problem hiding this comment.
When a string representing a valid numeric literal (for example, "300") is compared to a number,
JavaScript first converts the string into its numeric equivalent before performing the comparison.
As a result, the expression 20 < "300" evaluates to true.
To test if the function can correctly ignore non-numeric values,
consider including a string such as "300" in the relevant test cases.
| test("given an array with decimal/float number, returns the correct total sum", () => { | ||
| expect(sum([1, 2, 3, 1.5, 2.1])).toEqual(9.6); | ||
| }); |
There was a problem hiding this comment.
Decimal numbers in most programming languages (including JS) are internally represented in "floating point number" format. Floating point arithmetic is not exact. For example, the result of 46.5678 - 46 === 0.5678 is false because 46.5678 - 46 only yield a value that is very close to 0.5678. Even changing the order in which the program add/subtract numbers can yield different values.
So the following could happen
expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.805 ); // This fail
expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.8049999999999997 ); // This pass
expect( 0.005 + 0.6 + 1.2 ).toEqual( 1.8049999999999997 ); // This fail
console.log(1.2 + 0.6 + 0.005 == 1.805); // false
console.log(1.2 + 0.6 + 0.005 == 0.005 + 0.6 + 1.2); // falseCan you find a more appropriate way to test a value (that involves decimal number calculations) for equality?
Suggestion: Look up
- Checking equality in floating point arithmetic in JavaScript
- Checking equality in floating point arithmetic with Jest
cjyuan
added
Reviewed
2 participants
Self checklist